Skip to content

Enable compile/link time profiling#410

Open
greenc-FNAL wants to merge 3 commits intomainfrom
maintenance/enable-build-profiling
Open

Enable compile/link time profiling#410
greenc-FNAL wants to merge 3 commits intomainfrom
maintenance/enable-build-profiling

Conversation

@greenc-FNAL
Copy link
Contributor

No description provided.

@greenc-FNAL
Copy link
Contributor Author

@phlexbot cmake-fix

@github-actions
Copy link
Contributor

Automatic cmake-format fixes pushed (commit c2b60b7).
⚠️ Note: Some issues may require manual review and fixing.

@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #410      +/-   ##
==========================================
- Coverage   84.43%   84.40%   -0.04%     
==========================================
  Files         127      127              
  Lines        3329     3329              
  Branches      564      564              
==========================================
- Hits         2811     2810       -1     
  Misses        325      325              
- Partials      193      194       +1     
Flag Coverage Δ
unittests 84.40% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d7eb1a...a38525d. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in CMake configuration to profile build-time behavior (compile and link), helping developers identify expensive translation units and link steps during Phlex builds.

Changes:

  • Introduces ENABLE_BUILD_PROFILING CMake option to wrap compile/link rules with cmake -E time.
  • Enables compile time tracing (-ftime-trace) for Clang/GNU toolchains when profiling is enabled.
  • Enables additional LLD link tracing/statistics flags when LLD is detected.

@greenc-FNAL greenc-FNAL force-pushed the maintenance/enable-build-profiling branch from c2b60b7 to b0e703f Compare March 16, 2026 16:08
@greenc-FNAL
Copy link
Contributor Author

Review the full CodeQL report for details.

@greenc-FNAL greenc-FNAL force-pushed the maintenance/enable-build-profiling branch from b0e703f to ec28a53 Compare March 16, 2026 19:17
@knoepfel
Copy link
Member

@greenc-FNAL, is this comment by Copilot relevant? If not, please resolve the conversation.

@greenc-FNAL
Copy link
Contributor Author

@greenc-FNAL, is this comment by Copilot relevant? If not, please resolve the conversation.

It looks relevant at first blush, although the linker's map file writer might be parallel-safe. I'll try to look later; sorry for false start.

greenc-FNAL and others added 3 commits March 18, 2026 14:15
Remove --Map=phlex_link.map from the global LLD link options: since
multiple targets are linked in parallel, using a hardcoded filename
would cause all link jobs to race to write the same file, resulting
in corrupt or overwritten output. The --time-trace option already
generates a per-target trace file (appending .time-trace to the
output file name), and --print-archive-stats=- writes to stdout, so
neither of those has this issue.

Also correct an if(...) formatting issue caught by gersemi.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@greenc-FNAL greenc-FNAL force-pushed the maintenance/enable-build-profiling branch from ec28a53 to a38525d Compare March 18, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants